Updated quant deploy script for no-database deploys. #1452
Conversation
WalkthroughRefactors provisioning to always invoke Changes
Sequence Diagram(s)sequenceDiagram
%%{init: {"themeVariables":{"actorBorder":"#2b6cb0","actorBg":"#e6f2ff","noteBorder":"#805ad5","noteBg":"#f6f0ff"}}}%%
participant Entrypoint as CLI Entrypoint
participant Env as Environment (.env / Lagoon)
participant Drevops as ./scripts/drevops/provision.sh
rect rgba(98,147,255,0.06)
Entrypoint->>Env: read env vars (LAGOON_*, DREVOPS_*, .env)
alt DREVOPS_PROVISION_USE_PROFILE == "1"
Entrypoint->>Env: extract & export DREVOPS_PROVISION_OVERRIDE_DB
end
alt production env detected
Entrypoint->>Entrypoint: export DRUPAL_UNBLOCK_ADMIN=0, DREVOPS_PROVISION_SANITIZE_DB_SKIP=1
end
end
rect rgba(88,199,123,0.06)
Entrypoint->>Drevops: invoke ./scripts/drevops/provision.sh
Drevops-->>Entrypoint: status / completion
end
rect rgba(220,38,38,0.04)
note right of Entrypoint: Removed delegation to `scripts/quant/provision-quant.sh`
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-deploy.yml (1)
41-78: Workflow logic is sound; minor static analysis improvements recommended.The conditional output overrides are correctly structured:
- Branch-specific handling (quant-cloud-migration, project/quant-provision, default)
- Consistent image_suffix normalization across all branches
- Proper export of
skip_db_syncfor downstream useStatic analysis tools flagged quoting and consolidation opportunities in the shell script. While GitHub Actions provides a safe context for variable expansion, addressing these recommendations will improve robustness:
Optional improvement 1: Consolidate multiple
$GITHUB_OUTPUTredirects.Consider using a block redirect to reduce redundancy and improve clarity:
- elif [[ "${{ github.ref }}" == "refs/heads/project/quant-provision" ]]; then - echo "image_suffix=${{ steps.init.outputs.image_suffix }}" >> $GITHUB_OUTPUT - suffix="${{ steps.init.outputs.image_suffix }}" - clean_suffix="${suffix#-}" - echo "image_suffix_clean=$clean_suffix" >> $GITHUB_OUTPUT - echo "is_production=false" >> $GITHUB_OUTPUT - echo "environment_name=${{ steps.init.outputs.environment_name }}" >> $GITHUB_OUTPUT - echo "environment_exists=${{ steps.init.outputs.environment_exists }}" >> $GITHUB_OUTPUT - skip_db_sync="true" - echo "Overriding outputs for project/quant-provision branch: skipping database sync" + elif [[ "${{ github.ref }}" == "refs/heads/project/quant-provision" ]]; then + suffix="${{ steps.init.outputs.image_suffix }}" + clean_suffix="${suffix#-}" + skip_db_sync="true" + { + echo "image_suffix=${{ steps.init.outputs.image_suffix }}" + echo "image_suffix_clean=$clean_suffix" + echo "is_production=false" + echo "environment_name=${{ steps.init.outputs.environment_name }}" + echo "environment_exists=${{ steps.init.outputs.environment_exists }}" + } >> $GITHUB_OUTPUT + echo "Overriding outputs for project/quant-provision branch: skipping database sync"Apply the same pattern to lines 47–54 and 66–76 for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.docker/entrypoints/cli/03-provision-site.sh(1 hunks).github/workflows/build-deploy.yml(5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/build-deploy.yml
43-43: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:14:65: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:17:46: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:18:33: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:19:73: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:20:77: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:25:65: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:29:46: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:30:67: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:31:73: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:32:77: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:35:38: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:6:34: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:7:39: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:8:32: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:9:41: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2129:style:17:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
43-43: shellcheck reported issue in this script: SC2129:style:29:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
43-43: shellcheck reported issue in this script: SC2129:style:6:3: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: commit
🔇 Additional comments (2)
.docker/entrypoints/cli/03-provision-site.sh (1)
32-39: Clarify comment that contradicts the code execution.The comment on lines 32–34 states "The standard DrevOps provision script is not compatible with Quant Cloud," yet the code directly invokes
./scripts/drevops/provision.sh. This is contradictory and suggests the comment is stale. Either the DrevOps script has been updated to support Quant Cloud, or the comment requires updating to explain what changed.Has the DrevOps provisioning script been updated to work with Quant Cloud (e.g., to skip rsync or Lagoon-specific operations)? If so, please update the comment to reflect the current state. For example:
-# Delegate Drupal provisioning to the Quant-aware script. The standard -# DrevOps provision script is not compatible with Quant Cloud because it relies -# on Lagoon-specific tooling (e.g., rsync to Lagoon SSH). +# Delegate Drupal provisioning to the DrevOps script. +# The script now supports Quant Cloud by skipping Lagoon-specific operations via environment variables..github/workflows/build-deploy.yml (1)
141-141: Database sync condition correctly checks skip_db_sync flag.The added condition
&& steps.override-outputs.outputs.skip_db_sync != 'true'ensures database synchronization is skipped for the quant-provision branch as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/drevops/provision.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-push
- GitHub Check: commit
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.docker/entrypoints/cli/03-provision-site.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
.docker/entrypoints/cli/03-provision-site.sh
[warning] 24-24: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-push
- GitHub Check: commit
🔇 Additional comments (2)
.docker/entrypoints/cli/03-provision-site.sh (2)
8-14: Approve: Production environment handling is appropriate.The production environment block correctly sets safety-first flags (
DRUPAL_UNBLOCK_ADMIN=0andDREVOPS_PROVISION_SANITIZE_DB_SKIP=1) to prevent risky operations in production. The condition logic is sound.
26-26: Approve: Unconditional provisioning call.Removing the fallback logic and delegating all provisioning to
./scripts/drevops/provision.shsimplifies the flow and aligns with the PR objective of consolidating provisioning. Ensure that the drevops provision script is always present and handles all deployment scenarios (including no-database deploys as noted in the PR title).Verify that
./scripts/drevops/provision.shcorrectly handles:
- The new
DREVOPS_PROVISION_USE_PROFILEandDREVOPS_PROVISION_OVERRIDE_DBenvironment variables.- No-database deployment scenarios (as indicated by the PR title).
- Production environment safety flags set on lines 11–13.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-deploy.yml (1)
41-51: Output propagation logic is correct, but step name may be misleading.The refactored logic correctly propagates init outputs and transforms
image_suffixconsistently for all branches. However, the step name "Override outputs for special branches" doesn't accurately reflect the logic anymore—there's no visible conditional branch handling.Consider renaming to something like "Propagate and transform init outputs" for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-deploy.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-push
- GitHub Check: commit
🔇 Additional comments (1)
.github/workflows/build-deploy.yml (1)
13-13: Branch trigger addition looks good.Adding
project/quant-provisionto watched branches aligns with the provisioning flow updates mentioned in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-push
- GitHub Check: commit
| # | ||
| # See https://docs.drevops.com/workflows/deploy | ||
| DREVOPS_DEPLOY_TYPES=lagoon | ||
| DREVOPS_DEPLOY_TYPES=none |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the full context of the .env file around line 127
echo "=== .env file context around line 127 ==="
sed -n '120,134p' .env
echo -e "\n=== Check if .env.local exists ==="
if [ -f .env.local ]; then
echo ".env.local exists, showing content:"
cat .env.local
else
echo ".env.local does not exist"
fi
echo -e "\n=== Search for DREVOPS_DEPLOY_TYPES in codebase ==="
rg -n "DREVOPS_DEPLOY_TYPES" --type-list | head -5
echo -e "\n=== Find files that reference DREVOPS_DEPLOY_TYPES ==="
rg -l "DREVOPS_DEPLOY_TYPES"Repository: civictheme/monorepo-drupal
Length of output: 1219
🏁 Script executed:
#!/bin/bash
# Examine the deploy.sh script that uses DREVOPS_DEPLOY_TYPES
echo "=== Content of scripts/drevops/deploy.sh ==="
wc -l scripts/drevops/deploy.sh
echo -e "\n=== First 100 lines of deploy.sh ==="
head -100 scripts/drevops/deploy.sh
echo -e "\n=== Search for DREVOPS_DEPLOY_TYPES usage in deploy.sh ==="
rg -n "DREVOPS_DEPLOY_TYPES" scripts/drevops/deploy.sh -A 5 -B 5
echo -e "\n=== Search for 'none' in deploy.sh ==="
rg -n "none" scripts/drevops/deploy.sh | head -20Repository: civictheme/monorepo-drupal
Length of output: 7630
🏁 Script executed:
#!/bin/bash
# Check git history for changes to DREVOPS_DEPLOY_TYPES
echo "=== Git log for .env changes ==="
git log --oneline -n 20 -- .env
echo -e "\n=== Git show most recent .env change ==="
git show HEAD:.env | sed -n '120,134p'
echo -e "\n=== Git diff for .env in current PR/branch ==="
git diff HEAD~1 .env 2>/dev/null | head -50 || echo "Cannot get diff (single commit or no previous version)"
echo -e "\n=== Check current branch ==="
git rev-parse --abbrev-ref HEAD
echo -e "\n=== Search for any documentation about deploy types ==="
find . -type f \( -name "*.md" -o -name "*.txt" \) -exec grep -l "DREVOPS_DEPLOY_TYPES" {} \; 2>/dev/null | head -10Repository: civictheme/monorepo-drupal
Length of output: 824
🏁 Script executed:
#!/bin/bash
# Check for CI/CD configuration files
echo "=== Look for CI/CD configuration files ==="
find . -maxdepth 2 -type f \( -name ".github" -o -name ".gitlab-ci*" -o -name ".circleci*" -o -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | head -20
echo -e "\n=== Check for GitHub workflows ==="
find . -path "*/.github/workflows/*" -type f 2>/dev/null | head -10
echo -e "\n=== Look for any references to deployment strategy in commit ==="
git show 3020b9e3 --stat 2>/dev/null | head -30
echo -e "\n=== Get full commit message for context ==="
git log -1 --format="%B" 3020b9e3 2>/dev/null
echo -e "\n=== Check if there are alternative deploy scripts ==="
ls -la scripts/drevops/deploy*.sh 2>/dev/null || echo "No deploy scripts found"
echo -e "\n=== Search for references to 'quant' deployment ==="
rg -i "quant.*deploy" --type sh | head -20Repository: civictheme/monorepo-drupal
Length of output: 2857
🏁 Script executed:
#!/bin/bash
# Check what the script does when DREVOPS_DEPLOY_PROCEED is not set
echo "=== Look for DREVOPS_DEPLOY_PROCEED usage ==="
rg -n "DREVOPS_DEPLOY_PROCEED" scripts/drevops/deploy.sh -B 3 -A 3
echo -e "\n=== Check if there's any README about deployment configuration ==="
find . -name "README*" -o -name "DEPLOY*" 2>/dev/null | xargs grep -l "DREVOPS_DEPLOY" 2>/dev/null | head -5
echo -e "\n=== Look at deploy.sh comments about valid values ==="
sed -n '20,30p' scripts/drevops/deploy.sh
echo -e "\n=== Check .env comments for deploy-related variables ==="
grep -n "DREVOPS_DEPLOY" .env | head -15Repository: civictheme/monorepo-drupal
Length of output: 1900
Now I have enough information to provide a comprehensive rewritten review comment. The original concern is substantively correct—this does disable all deployments—but the issue is more nuanced. The change appears intentional based on the commit message, but it uses an undocumented workaround instead of the proper mechanism.
Document the "none" deployment type and clarify the deployment strategy.
The value DREVOPS_DEPLOY_TYPES=none disables all deployments, but this value is not documented as valid in the script comments (which list only: artifact, docker, webhook, lagoon). While the commit "Remove lagoon deploy type" confirms this is intentional, using none is a workaround that bypasses the validation.
The deploy script has a proper failsafe mechanism: DREVOPS_DEPLOY_PROCEED=0 (documented as "a failsafe to prevent any deployments"). Consider using that instead, or:
- Update
scripts/drevops/deploy.shto documentnoneas a valid value in the comments (lines 23-25). - Update
.envto document thatnonedisables all deployments, or useDREVOPS_DEPLOY_PROCEED=0as the mechanism. - Clarify the deployment strategy: The PR mentions updating provisioning for "quant" deploys, but doesn't explain how sites will be deployed if all deployment types are disabled. Document whether this is permanent or environment-specific (e.g., development-only).
🤖 Prompt for AI Agents
.env around line 127: the entry DREVOPS_DEPLOY_TYPES=none disables all
deployments but is undocumented and bypasses the deploy script's intended
failsafe; either document "none" as a valid deploy type in
scripts/drevops/deploy.sh comments (update lines ~23-25 to include "none") and
add a comment in .env explaining that "none" disables all deployments and
whether this is environment-specific, or revert to the intended failsafe by
setting DREVOPS_DEPLOY_PROCEED=0 in .env; also update README/PR text to clarify
the deployment strategy (how/when "quant" sites will be deployed) and whether
this is a permanent or dev-only setting.
…ning.
Checklist before requesting a review
Issue #123456 by drupal_org_username: Issue titleChangedsection about WHY something was done if this was not a normal implementationChanged
Summary by CodeRabbit
Improvements
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.